Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove forward-sub for hw intrinsics during inlining #62883

Merged
merged 1 commit into from
Dec 17, 2021
Merged

Remove forward-sub for hw intrinsics during inlining #62883

merged 1 commit into from
Dec 17, 2021

Conversation

EgorBo
Copy link
Member

@EgorBo EgorBo commented Dec 16, 2021 •
edited
Loading

Fixes #60330

it needs a better forward-sub pass (I am planning to come up with a prototype soon) to properly substitute struct types, currently we might end up with:

               [000021] n----------- arg1         +--*  OBJ       simd12<System.Numerics.Vector3>
               [000020] ------------              |  \--*  ADDR      byref
               [000009] ------------              |     \--*  LCL_VAR

after fsub:

               [000021] n----------- arg1         +--*  OBJ       simd12<System.Numerics.Vector3>
               [000020] ------------              |  \--*  ADDR      byref
               [000009] ------------              |     \--*  HWINTRINSIC simd12 float get_Zero

while expected to be:

               [000009] ------------              +--*  HWINTRINSIC simd12 float get_Zero

cc @kunalspathak

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Dec 16, 2021
@ghost ghost assigned EgorBo Dec 16, 2021
@ghost
Copy link

ghost commented Dec 16, 2021

Tagging subscribers to this area: @JulieLeeMSFT
See info in area-owners.md if you want to be subscribed.

Issue Details

Fixes #60330

it needs a better forward-sub pass (I am planning to come up with a prototype soon) to properly substitute struct types, currently we might end up with:

               [000021] n----------- arg1         +--*  OBJ       simd12<System.Numerics.Vector3>
               [000020] ------------              |  \--*  ADDR      byref
               [000009] ------------              |     \--*  HWINTRINSIC simd12 float get_Zero

cc @kunalspathak

Author: EgorBo
Assignees: -
Labels:

area-CodeGen-coreclr
Milestone: -

@kunalspathak
Copy link
Member

So this reverts part of #55875? Curious to see asmdiff (when they are ready)

@kunalspathak
Copy link
Member

@dotnet/jit-contrib

@EgorBo
Copy link
Member Author

EgorBo commented Dec 16, 2021 •
edited
Loading

So this reverts part of #

Yes, but I am working on a forward sub for FPTR (function pointers) nodes locally and will handle this case too. For now I just want to unblock the CI. the diff is expected to be very small

@SingleAccretion
Copy link
Contributor

Fixes #62815 too (or was the issue number not quite correct in the PR description)?

@BruceForstall
Copy link
Member

Looks like few asm diffs: https://dev.azure.com/dnceng/public/_build/results?buildId=1516015&view=ms.vss-build-web.run-extensions-tab

@EgorBo EgorBo merged commit 5a5d379 into dotnet:main Dec 17, 2021
@kunalspathak
Copy link
Member

@EgorBo - could you confirm that this fixes #62815 as well and if yes, close it?

@SingleAccretion SingleAccretion mentioned this pull request Dec 20, 2021
Closed
@ghost ghost locked as resolved and limited conversation to collaborators Jan 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@BruceForstall BruceForstall BruceForstall approved these changes

Assignees

@EgorBo EgorBo

Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Assertion failed 'child->isContained()'
4 participants
@EgorBo @kunalspathak @SingleAccretion @BruceForstall